Skip to content

🌱 Use server side apply to update test resource#791

Merged
m1kola merged 1 commit intooperator-framework:mainfrom
m1kola:flake_cluster_extension_admission_test
Apr 29, 2024
Merged

🌱 Use server side apply to update test resource#791
m1kola merged 1 commit intooperator-framework:mainfrom
m1kola:flake_cluster_extension_admission_test

Conversation

@m1kola
Copy link
Copy Markdown
Member

@m1kola m1kola commented Apr 26, 2024 •
edited
Loading

Description

Deals with a flake such as this:

go test -tags upstream -v ./test/e2e/...
=== RUN   TestClusterExtensionPackageUniqueness
    cluster_extension_admission_test.go:30: create first resource
    cluster_extension_admission_test.go:42: create second resource with the same package as the first resource
    cluster_extension_admission_test.go:54: create second resource with different package
    cluster_extension_admission_test.go:66: update second resource with package which already exists on the cluster
    cluster_extension_admission_test.go:69: 
        	Error Trace:	/home/runner/work/operator-controller/operator-controller/test/e2e/cluster_extension_admission_test.go:69
        	Error:      	Error "Operation cannot be fulfilled on clusterextensions.olm.operatorframework.io \"test-extension-74hdj\": the object has been modified; please apply your changes to the latest version and try again" does not contain "Package \"package1\" is already installed via ClusterExtension \"test-extension-first\""
        	Test:       	TestClusterExtensionPackageUniqueness
--- FAIL: TestClusterExtensionPackageUniqueness (2.03s)

Was introduced in #785.

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

Use server side apply to update test resource
ab7ddfa 
Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 26, 2024
@netlify
Copy link
Copy Markdown

netlify bot commented Apr 26, 2024 •
edited
Loading

✅ Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit ab7ddfa
🔍 Latest deploy log https://app.netlify.com/sites/olmv1/deploys/662bb58d8376550008264509
😎 Deploy Preview https://deploy-preview-791--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@m1kola m1kola marked this pull request as ready for review April 26, 2024 14:10
@m1kola m1kola requested a review from a team as a code owner April 26, 2024 14:10
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 26, 2024
perdasilva
perdasilva reviewed Apr 26, 2024
View reviewed changes
perdasilva
perdasilva reviewed Apr 26, 2024
View reviewed changes
@m1kola m1kola requested a review from perdasilva April 26, 2024 15:16
perdasilva
perdasilva reviewed Apr 26, 2024
View reviewed changes
test/e2e/cluster_extension_admission_test.go

func TestClusterExtensionPackageUniqueness(t *testing.T) {
ctx := context.Background()
fieldOwner := client.FieldOwner("operator-controller-e2e")
Copy link
Copy Markdown
Contributor

@perdasilva perdasilva Apr 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using SSA is valid and maybe we should have a wider discussion about how best to use it in e2e tests. But, we shouldn't be seeing conflicts of the type in the issue coming out of our test code. If we do it's because we forgot to get the latest from the server after a create or update. SSA is nice because it makes things feel more natural for e2e tests. Do we need to respect any conventions when using SSA? Should we switch all mutating interactions in e2e over to SSA?

perdasilva
perdasilva approved these changes Apr 29, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@perdasilva perdasilva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm - on a general note we may want to consider best practices (if any) around SSA that we should respect in the e2es

@m1kola m1kola added this pull request to the merge queue Apr 29, 2024
Merged via the queue into operator-framework:main with commit e672804 Apr 29, 2024
@m1kola m1kola deleted the flake_cluster_extension_admission_test branch June 14, 2024 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@perdasilva perdasilva perdasilva approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@m1kola @perdasilva